feat(cli): generate section-1 man SYNOPSIS and OPTIONS from Typer - #1524
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e49576a209
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Address the chatgpt-codex review on basicmachines-co#1524: - Replace the speculative `getattr(param, "hidden", False)` fallback in render_cli_synopsis/render_options with a structural ClickParam/ClickCommand Protocol and read `param.hidden` directly, so an unexpected parameter shape fails fast instead of being silently treated as public (AGENTS.md: no speculative getattr). The Protocol stays structural, so no Click import is pulled into the lightweight `basic_memory.man` module. - Render mutually exclusive option pairs (--json/--plain, --local/--cloud) as a single `[--a | --b]` alternative in the shell SYNOPSIS, matching the CLI's own rejection of both flags together, instead of flattening them into freely combinable tokens. - Keep Click `multiple=True` options repeatable: `find --meta` renders `[--meta META ...]` again rather than degrading to `[--meta META]`. - Add `cli` to the shipped Manpage schema ownership enum and fix its residual `typer` prose, so a user copying the opt-in schema validates the regenerated section-1 pages without an enum-mismatch warning. Regenerated all 8 section-1 pages via scripts/update_man_pages.py; regeneration is idempotent. Added regression tests for the grouped mutex pairs, the repeatable `...` notation, and hidden-option exclusion. Refs basicmachines-co#610 Signed-off-by: FBISiri <[email protected]>
|
Status check-in, plus the bits that are actually mine to flag. This is the slice (B) implementation following your 09-07 direction: Rather than make you reverse-engineer +706/-98, the short review map:
One judgment call worth your eye, since it's the only spot where I picked semantics over mechanics: mutually-exclusive groups are declared explicitly, not inferred. So: would you rather the generator infer groups from Click metadata instead of relying on a declared list? That's a real design change and I'd much rather do it now than after merge. CI is 10/10 green on |
The eight bundled section-1 pages (apropos, cat, find, grep, head, ls, tail, tree) declared `generated: hand`, and their SYNOPSIS and OPTIONS blocks were hand restatements of the POSIX verb definitions in cli/commands/posix.py. They had already drifted: grep(1)'s SYNOPSIS listed --json/--plain/--project/--local/--cloud while its OPTIONS documented only -F and the pagination flags. Render both blocks from the Typer command tree and lock them with a byte-equality drift test, mirroring what basicmachines-co#1478 did for section-3 PARAMETERS from the MCP registry: - man/__init__.py: render_cli_synopsis()/render_options() from a resolved Click command; _SYNOPSIS_BODY_RE/_OPTIONS_RE + extract/replace helpers; declare_ownership(text, owner=...) with declare_registry_ownership kept as a thin wrapper. - scripts/update_man_pages.py: a section-1 branch in the existing `just man-regen` pipeline; resolve_cli_command() walks the command tree (apropos maps to `bm man apropos`). - tests/test_man_pages.py: drift test, ownership, shared/global flags in OPTIONS, alias + paired-boolean rendering, block-scoped replacement. Per the maintainer's call (basicmachines-co#610), the ownership token is `generated: cli`. OPTIONS renders the complete public option list including the shared output/routing flags (grep(1) grows from four bullets to the full set), and preserves CLI syntax PARAMETERS has no concept of: flag aliases (`-F, --literal`) and paired booleans (`--frontmatter / --no-frontmatter`). Curated sections (NAME, DESCRIPTION, EXAMPLES, SEE ALSO) stay byte-identical and hand-owned; the groff sources are out of scope. Refs basicmachines-co#610 Signed-off-by: FBISiri <[email protected]>
Address the chatgpt-codex review on basicmachines-co#1524: - Replace the speculative `getattr(param, "hidden", False)` fallback in render_cli_synopsis/render_options with a structural ClickParam/ClickCommand Protocol and read `param.hidden` directly, so an unexpected parameter shape fails fast instead of being silently treated as public (AGENTS.md: no speculative getattr). The Protocol stays structural, so no Click import is pulled into the lightweight `basic_memory.man` module. - Render mutually exclusive option pairs (--json/--plain, --local/--cloud) as a single `[--a | --b]` alternative in the shell SYNOPSIS, matching the CLI's own rejection of both flags together, instead of flattening them into freely combinable tokens. - Keep Click `multiple=True` options repeatable: `find --meta` renders `[--meta META ...]` again rather than degrading to `[--meta META]`. - Add `cli` to the shipped Manpage schema ownership enum and fix its residual `typer` prose, so a user copying the opt-in schema validates the regenerated section-1 pages without an enum-mismatch warning. Regenerated all 8 section-1 pages via scripts/update_man_pages.py; regeneration is idempotent. Added regression tests for the grouped mutex pairs, the repeatable `...` notation, and hidden-option exclusion. Refs basicmachines-co#610 Signed-off-by: FBISiri <[email protected]>
Signed-off-by: phernandez <[email protected]>
d793482 to
4cc534d
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review Please review current head 4cc534d. Both contributor commits are preserved after rebasing onto current main; all four earlier findings are verified and resolved. The maintainer accepts the explicit mutex declaration. Local tests and exact commands are in the updated description. Do not merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cc534d3e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 948de9a488
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please re-review unchanged head 948de9a after the evidence reply on the grep thread. Its independent option syntax is pre-existing on base ffbb6e9; the requirement/help, curated mode description, and examples remain present. The find regression has been fixed with two generated forms. Keep the approved explicit-declaration design and focused compatibility scope. All checks pass; no merge requested. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 948de9a488
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <[email protected]>
2 participants
Why
Section-1 manual SYNOPSIS and OPTIONS blocks duplicated the CLI definitions and had drifted, omitting public output and routing flags. This implements the section-1 generation slice of #610.
What Changed
Generate these blocks for all eight bundled section-1 pages from the Typer command tree, declare
generated: cli, and validate that ownership token in the shipped Manpage schema. Preserve aliases, paired booleans, repeatable options, and the full public option list.Original implementation and review fixes by @FBISiri. Both contributor commits remain separate, with authorship and sign-offs preserved, rebased onto
origin/mainatffbb6e9d66b6db73aafa370f5a5386bff3cb9186. The maintainer follow-up retains cat's line/section incompatibility and follow-up range guidance in its curated DESCRIPTION.Implementation Details
scripts/update_man_pages.pyresolves section-1 commands and uses the renderers inbasic_memory.man; section-3 registry generation continues through the same pipeline. Structural parameter protocols replace speculative visibility access.The current-head review follow-up restores find's separate listing and metadata synopsis forms. Metadata mode requires one
--metaand permits repeats plus--fields; listing mode carries--nameand--depth. These mode constraints are explicit, while option spellings continue to come from Typer. A regression test verifies both forms and their page regeneration round-trip.The maintainer accepts the contributor's explicit mutex declaration:
--json | --plainand--local | --cloud. The review follow-up also restores the base cat synopsis constraint as[--lines LINES | --section SECTION], using the same declaration mechanism with value placeholders. Groups are not inferred.--projectand--project-idremain independently documented because the ID takes precedence. Future mutex pairs must be declared explicitly. Curated sections remain hand-owned; groff sources are unchanged.Testing
uv run pytest tests/test_man_pages.py tests/cli/test_man_command.py tests/cli/test_cli_man_lookup.py -q --no-cov— 88 passed after the find-mode and cat-alternative fixes.uv run pytest tests/mcp/test_tool_posix.py tests/cli/test_cli_posix_verbs.py -k find -q --no-cov— 104 passed, 295 deselected.just package-check— passed across Claude Code, Codex, shared skills, Hermes, OpenClaw, Tau, and Pi, including generated skill-reference drift checks.uv sync --extra milvusfollowed byjust typecheck— passed. The initial typecheck lacked the optionalpymilvusdependency in the new worktree environment.just man-regen && git diff --exit-code— passed; generated pages match their sources, including after the curated cat description fix.uv run ruff check src/basic_memory/man/__init__.py scripts/update_man_pages.py tests/test_man_pages.py— passed.uv run ruff format --check src/basic_memory/man/__init__.py scripts/update_man_pages.py tests/test_man_pages.py— passed.just doctor— passed.git range-diff 368e60762..d793482d1821da88eb7c39050ad41a0ceddcd9c1 origin/main..8b03e94135374d96d457863ec17cc2b3bdc47b74— both contributor patches unchanged by rebase.Risks / Follow-ups
The explicit mutex list requires maintenance when new cross-option constraints are introduced. This change affects manual generation, not command execution. Full SQLite/Postgres suites were not run locally; targeted manual/CLI tests and package verification passed. No merge is requested by this update.